feat(metrics): add optional k8s metadata labels to rules_matches#3839
feat(metrics): add optional k8s metadata labels to rules_matches#3839Debasish-87 wants to merge 1 commit intofalcosecurity:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Debasish-87 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @Debasish-87! It looks like this is your first PR to falcosecurity/falco 🎉 |
12ad7b4 to
56c6a51
Compare
Signed-off-by: Debasish-87 <22btics06@suiit.ac.in>
56c6a51 to
314a35a
Compare
|
Hi maintainers, This PR adds an opt-in configuration to include Kubernetes metadata in Prometheus metrics while preserving backward compatibility and avoiding high-cardinality issues. Would appreciate your feedback and review. Thanks! |
|
Hi reviewers, gentle ping for review. Happy to make any changes if needed. |
Hey @Debasish-87 Thanks for this PR and sorry for the delay. I just want to let you know that it's under my radar. |
There was a problem hiding this comment.
Hey @Debasish-87, thanks for looking into this. The feature request in #3826 is valid, and including namespace/pod labels in Prometheus metrics is standard practice in the Kubernetes ecosystem (see Prometheus instrumentation best practices).
However, I believe this PR doesn't address the issue. It adds k8s_ns_name and k8s_pod_name labels hardcoded to "n/a", which provides no useful information. Constant-value labels violate Prometheus best practices since labels should distinguish time series.
The underlying problem is that rules_matches_total is currently a per-rule aggregate counter. The stats_manager::on_event() only receives a const falco_rule& and has no access to per-event data. That said, the sinsp_evt* IS available at match time (in falco_engine::process_event()), and falco_outputs::handle_event() already resolves all output fields (including k8s.ns.name, k8s.pod.name from the container plugin) into a map<string, string>. So the data is there, it just doesn't reach the stats_manager.
A more generic solution would be to allow users to configure which output fields should be promoted to Prometheus labels on rules_matches_total. Something like:
metrics:
rules_counters_extra_labels:
- k8s.ns.name
- k8s.pod.nameThis would require changes to the stats_manager to track per-(rule, label_values) counters instead of just per-rule counters, but the architecture supports it since the event data is available at the right point in the pipeline.
This is a larger change than what this PR proposes, and I'm still unsure if this is a good idea. I'd suggest opening a design discussion for this approach. wdyt? 🤔
cc @falcosecurity/core-maintainers
What type of PR is this?
/kind feature
Any specific area of the project related to this PR?
/area engine
What this PR does / why we need it:
This PR introduces an optional configuration flag
metrics.include_k8s_metadatato enrich thefalcosecurity_falco_rules_matches_totalPrometheus metric with Kubernetes-related labels.When enabled, the metric includes:
k8s_ns_namek8s_pod_nameThis addresses the current limitation where Kubernetes context is available in Falco event logs but not exposed in Prometheus metrics, making it difficult to correlate alerts with specific workloads.
Since Kubernetes metadata is not available at the metrics aggregation layer, placeholder values (
"n/a") are used. This provides a consistent interface while preserving forward compatibility for future improvements where real metadata may be available.The feature is disabled by default to avoid introducing high-cardinality metrics unless explicitly enabled by the user.
Which issue(s) this PR fixes:
Fixes #3826
Special notes for your reviewer:
false)Does this PR introduce a user-facing change?: